ci(publish-to-npm): Download and extract build as single artifact at the correct path (fixes #48). #49
Conversation
WalkthroughThe pull request modifies the GitHub Actions workflow for npm publishing in Changes
Possibly Related PRs
Suggested Reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
junhaoliao
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/publish-to-npm.yml (1)
37-39: Consider adding post-download verification.While the artifact download configuration matches the upload settings, consider adding these reliability improvements:
Add a verification step after the download:
- uses: "actions/download-artifact@v4" with: name: "dist" path: "dist/" +- name: "Verify downloaded artifacts" + run: | + ls -la dist/ + if [ ! -d "dist" ] || [ -z "$(ls -A dist/)" ]; then + echo "Error: dist directory is empty or missing" + exit 1 + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/publish-to-npm.yml(2 hunks)
🔇 Additional comments (1)
.github/workflows/publish-to-npm.yml (1)
26-29: LGTM! Verify the artifact path format.
The artifact upload configuration looks good with explicit naming and error handling. The 1-day retention period is appropriate for CI artifacts.
Let's verify the path format and presence of the dist directory:
✅ Verification successful
Let me check the actual content of these files to understand how the dist directory is used.
The artifact path format "dist/" is correct and properly aligned with the project structure
The path format "dist/" in the artifact upload configuration is verified to be correct because:
- The
package.jsoncorrectly references the dist directory for npm package files and exports - The
Taskfile.ymldefinesG_DIST_DIRvariable that's used in the build process - The workflow shows a complete build-and-publish pipeline where:
task packagegenerates the build artifacts- The artifacts are uploaded from "dist/"
- The same path is used for downloading in the publish job
- The path aligns with npm publish expectations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the dist directory path and its usage in the codebase
# Check if dist is mentioned in any task definitions or build scripts
rg -l "dist" package.json Taskfile.yml 2>/dev/null || echo "No dist references found in build files"
# Look for any existing dist directory patterns
fd -t d "dist$" || echo "No dist directory found"
Length of output: 82
Script:
#!/bin/bash
# Check the content of package.json and Taskfile.yml for dist-related configurations
echo "=== package.json content ==="
rg "dist" package.json -C 2
echo -e "\n=== Taskfile.yml content ==="
rg "dist" Taskfile.yml -C 2
# Check the workflow file for build steps
echo -e "\n=== Workflow file content ==="
cat .github/workflows/publish-to-npm.yml
Length of output: 1929
|
Alternatively, this config would also work though it is less straight forward: |
.github/workflows/publish-to-npm.yml
Outdated
| @@ -23,6 +23,7 @@ jobs: | |||
| - name: "Upload build artifacts" | |||
| uses: "actions/upload-artifact@v4" | |||
| with: | |||
| name: "dist" | |||
There was a problem hiding this comment.
This is optional, but do you want to call it something more readable like clp-ffi-js or clp-ffi-js-dist so that it's obvious when downloaded?
There was a problem hiding this comment.
if you mean where a developer downloads the artifacts from the job's Summary page, sure
junhaoliao
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/publish-to-npm.yml (1)
40-42: Consider adding explicit error handling for artifact downloadWhile the configuration is correct, consider adding explicit error handling to ensure the workflow fails gracefully if the artifact download fails.
Add the following configuration:
with: name: "${{env.G_CLP_FFI_JS_DIST_ARTIFACT_NAME}}" path: "dist/" + fail-on-error: true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/publish-to-npm.yml(3 hunks)
🔇 Additional comments (2)
.github/workflows/publish-to-npm.yml (2)
29-32: LGTM! Robust artifact upload configuration
The artifact upload configuration includes all necessary safeguards:
- Proper error handling with
if-no-files-found: "error" - Appropriate retention period for CI/CD context
- Correct path specification
8-10: LGTM! Environment variable follows naming best practices
The environment variable definition is well-structured:
- Uses a descriptive prefix "clp-ffi-js-dist" for clarity
- Incorporates GitHub SHA for uniqueness
- Follows the suggestion from previous reviews for better readability
Let's verify the naming convention across the workflow:
✅ Verification successful
Let me verify the artifact naming usage in more detail:
Artifact naming convention is consistently implemented
The environment variable G_CLP_FFI_JS_DIST_ARTIFACT_NAME is properly referenced throughout the workflow:
- Defined at the workflow level for global accessibility
- Consistently used in both upload and download artifact steps
- Maintains uniqueness with SHA while keeping the name readable
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent artifact naming pattern across the workflow
# Expected: Only references to the new environment variable for artifact names
rg -i 'name:.*artifact' .github/workflows/publish-to-npm.yml
Length of output: 219
Script:
#!/bin/bash
# Check all references to the artifact name variable and related patterns
rg -A 2 'G_CLP_FFI_JS_DIST_ARTIFACT_NAME|artifact' .github/workflows/publish-to-npm.yml
Length of output: 540
Related: #48
Description
Validation performed
ls -l distin the debug job showed all the artifacts: https://github.com/junhaoliao/clp-ffi-js/actions/runs/12554958088/job/35004441486Summary by CodeRabbit